Skip to content

feat(tags): implement tags dropdown in Add/Edit Task dialogs#351

Closed
ShivaGupta-14 wants to merge 1 commit intoCCExtractor:mainfrom
ShivaGupta-14:feat/210-tag-dropdown-selection
Closed

feat(tags): implement tags dropdown in Add/Edit Task dialogs#351
ShivaGupta-14 wants to merge 1 commit intoCCExtractor:mainfrom
ShivaGupta-14:feat/210-tag-dropdown-selection

Conversation

@ShivaGupta-14
Copy link
Contributor

@ShivaGupta-14 ShivaGupta-14 commented Dec 30, 2025 •
edited
Loading

Description

  • Create SearchAndAddSelector component with search, select, and create functionality
  • Use local Popover (no Portal) to fix click-through issue in Dialog without modifying global popover.tsx
  • Display selected tags as removable chips with X buttons
  • Add search filtering for existing tags
  • Add inline "Create new tag" option for non-existing tags
  • Integrate SearchAndAddSelector into TaskDialog and AddTaskDialog

Popover-in-Dialog fix (local component approach):

  • Created LocalPopoverContent inside SearchAndAddSelector that renders without Portal
  • This avoids the click-through issue when Popover is used inside Dialog
  • No modifications to global popover.tsx, keeps Shadcn components untouched
  • Component is reusable for annotations and other features

Tests:

  • Add comprehensive tests for SearchAndAddSelector component

  • Update AddTaskDialog tests for new tag selection flow

  • Update TaskDialog tests for tag editing with SearchAndAddSelector

  • Mock SearchAndAddSelector in tests

  • Fixes: Reusable tags for Editing or Adding a task #210

Bug: Tag removal not persisting (Fixed in this PR)

  • Video:
Screen.Recording.2026-01-12.at.12.50.58.AM.mov

Bug fix:

  • Fix tag removal not persisting: now sends tags with "-" prefix to backend for removals
  • Updated handleSaveTags to calculate tag additions and removals correctly

Checklist

  • Ran npx prettier --write . (for formatting)
  • Ran gofmt -w . (for Go backend)
  • Ran npm test (for JS/TS testing)
  • Added unit tests, if applicable
  • Verified all tests pass
  • Updated documentation, if needed

Additional Notes

Demo Video:

  • Instant UI update on tag removal (for better UX)
Screen.Recording.2026-01-12.at.12.31.45.AM.mov

Screenshots:

  • Before/After (improved UI):
Screenshot 2026-01-12 at 1 20 21 AM Screenshot 2026-01-12 at 1 22 39 AM Screenshot 2026-01-12 at 1 21 48 AM Screenshot 2026-01-12 at 1 24 07 AM Screenshot 2026-01-12 at 1 24 02 AM

@github-actions
Copy link

github-actions bot commented Dec 30, 2025

Thank you for opening this PR!

Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools.

Please take a moment to:

  • Check the "Files changed" tab
  • Leave comments on any lines for functions, comments, etc. that are important, non-obvious, or may need attention
  • Clarify decisions you made or areas you might be unsure about and/or any future updates being considered.
  • Finally, submit all the comments!

More information on how to conduct a self review:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request

This helps make the review process smoother and gives us a clearer understanding of your thought process.

Once you've added your self-review, we'll continue from our side. Thank you!

ShivaGupta-14
ShivaGupta-14 commented Dec 30, 2025
View reviewed changes
ShivaGupta-14
ShivaGupta-14 commented Dec 30, 2025
View reviewed changes
Copy link
Contributor Author

@ShivaGupta-14 ShivaGupta-14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self review done! ready for review

@ShivaGupta-14
Copy link
Contributor Author

ShivaGupta-14 commented Dec 30, 2025

Hi @its-me-abhishek!

I’ve added dropdown tests in AddTaskDialog.test.tsx since the logic is identical in both dialogs, for TaskDialog.test.tsx, I only kept the remove/save tests to avoid duplication, should I also add the same dropdown interaction tests there, or is covering it once sufficient? Let me know your preference.

its-me-abhishek
its-me-abhishek requested changes Dec 31, 2025
View reviewed changes
Copy link
Collaborator

@its-me-abhishek its-me-abhishek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please look into the above comments

@ShivaGupta-14
Copy link
Contributor Author

ShivaGupta-14 commented Jan 6, 2026

on it

@ShivaGupta-14 ShivaGupta-14 force-pushed the feat/210-tag-dropdown-selection branch from 06dcaa3 to d255e26 Compare January 6, 2026 19:06
@ShivaGupta-14
Copy link
Contributor Author

ShivaGupta-14 commented Jan 6, 2026

done with all changes, ready for review

its-me-abhishek
its-me-abhishek requested changes Jan 7, 2026
View reviewed changes
Copy link
Collaborator

@its-me-abhishek its-me-abhishek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested some changes and fixes

@ShivaGupta-14
Copy link
Contributor Author

ShivaGupta-14 commented Jan 8, 2026

@its-me-abhishek, I understand your suggestions, but before proceeding, I’d genuinely like to get your input. I’ve also opened a PR for the same issue where I implemented a TagSelector component with search and multi-select functionality that shows the selected items. As a user, I personally prefer this approach, but I’d really appreciate your suggestion. I’m happy to proceed with whatever you decide.

@ShivaGupta-14 ShivaGupta-14 force-pushed the feat/210-tag-dropdown-selection branch from d255e26 to 20f9279 Compare January 11, 2026 17:26
@ShivaGupta-14
Copy link
Contributor Author

ShivaGupta-14 commented Jan 11, 2026 •
edited
Loading

I implemented the current solution with considering all things in mind(your review and user experience), but i faced a issue with this ->

Issue faced:

the Popover inside Dialog had a click-through issue. radix UI's Dialog overlay blocks pointer events to Popover content because both use Portal and render at level with the same z-index.

Approaches explored:

  • z-index adjustments -> didn't work, the overlay intercepts events regardless of z-index

  • modal={false} on Dialog -> works, but breaks expected modal behavior (background scrolls, focus escapes)

  • modal={true} on Popover -> Clicks work, but input doesn't receive focus due to focus trap conflict

  • Working Solutions:

  • Option 1: Remove Portal (Simple)
    remove Primitive.Portal wrapper from popover.tsx, just (1 file change, 2 lines)
    Nothing else changes for this option. Just delete the opening and closing
    Risk: May clip if parent has overflow: hidden

  • Option 2: Container Solution (Current implementation)
    Pass container ref from Dialog to Popover (4 files changed, little bit complex to understand in first go)
    no clipping risk, portal behavior preserved

References:

Stackover link: Link
GIthub issue: Link

Current Status

Implemented Container Solution. All tests passing.
Which approach do you prefer?

@ShivaGupta-14
Copy link
Contributor Author

ShivaGupta-14 commented Jan 11, 2026 •
edited
Loading

also we can do one more thing if going with option 1st -> to maintain two popover file (one with portal and one without portal)

ShivaGupta-14
ShivaGupta-14 commented Jan 11, 2026
View reviewed changes
ShivaGupta-14
ShivaGupta-14 commented Jan 11, 2026
View reviewed changes
@ShivaGupta-14 ShivaGupta-14 force-pushed the feat/210-tag-dropdown-selection branch from 20f9279 to f96c5db Compare January 11, 2026 19:13
ShivaGupta-14
ShivaGupta-14 commented Jan 11, 2026
View reviewed changes
Copy link
Contributor Author

@ShivaGupta-14 ShivaGupta-14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@its-me-abhishek self review done, ready for review

@ShivaGupta-14 ShivaGupta-14 force-pushed the feat/210-tag-dropdown-selection branch from f96c5db to a57865a Compare January 11, 2026 20:52
@ShivaGupta-14
feat(tags): implement multi-select tag component for Add/Edit Task di…
46cd01d 
…alogs

- Create SearchAndAddSelector component with search, select, and create functionality
- Use local Popover (no Portal) to fix click-through issue in Dialog without modifying global popover.tsx
- Display selected tags as removable chips with X buttons
- Add search filtering for existing tags
- Add inline "Create new tag" option for non-existing tags
- Integrate SearchAndAddSelector into TaskDialog and AddTaskDialog

Bug fix:
- Fix tag removal not persisting: now sends tags with "-" prefix to backend for removals
- Updated handleSaveTags to calculate tag additions and removals correctly

Tests:
- Add comprehensive tests for SearchAndAddSelector component
- Update AddTaskDialog tests for new tag selection flow
- Update TaskDialog tests for tag editing with SearchAndAddSelector
- Mock SearchAndAddSelector in tests

Fixes: CCExtractor#210
@ShivaGupta-14 ShivaGupta-14 force-pushed the feat/210-tag-dropdown-selection branch from a57865a to 46cd01d Compare January 12, 2026 16:50
@ShivaGupta-14
Copy link
Contributor Author

ShivaGupta-14 commented Jan 12, 2026

@its-me-abhishek thank you for the thoughtful guidance on the channel! Following your suggestion about keeping Shadcn components untouched (for long-term maintainability):

Instead of modifying the global Popover or passing complex refs, I’d suggest to keep the core UI components as-is. If the Popover-in-Dialog issue persists, extend the existing Popover into a separate, local component for this feature. Combine the modified logic with the SearchAndAddSelector by renaming it so it’s reusable for annotations and other parts of the app without touching the global files.

I've refactored the implementation:

  • Created a local SearchAndAddSelector component that handles the Popover internally (no Portal, fixes click issue and good UI)
  • Kept popover.tsx completely untouched
  • Made the component reusable for other features like annotations

All tests passing, happy to make any adjustments based on your review. thanks again for the clear direction, learned a lot from this!

Also, if @Hell1213's PR #390 looks good for the project, happy to go with that approach instead! :)

@ShivaGupta-14
Copy link
Contributor Author

ShivaGupta-14 commented Jan 14, 2026

Hi @its-me-abhishek, just a request to check this PR whenever you have time. The Popover changes have been refactored as per your suggestion, so I’d really appreciate your feedback whenever you get a chance.

@its-me-abhishek
Copy link
Collaborator

its-me-abhishek commented Jan 17, 2026

@ShivaGupta-14 this got fixed by #390, hence, closing the PR

@ShivaGupta-14 ShivaGupta-14 deleted the feat/210-tag-dropdown-selection branch February 7, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@its-me-abhishek its-me-abhishek Awaiting requested review from its-me-abhishek

Assignees

No one assigned

Labels

in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Reusable tags for Editing or Adding a task

2 participants

@ShivaGupta-14 @its-me-abhishek